-
Notifications
You must be signed in to change notification settings - Fork 228
feat(save-user-data): extend user data abstract class for api backend COMPASS-9558 #7114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Merge branch 'user-data-interface' of https://github.com/mongodb-js/compass into user-data-interface
Merge branch 'extend-user-data' of https://github.com/mongodb-js/compass into extend-user-data
} | ||
|
||
return true; | ||
} catch (error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the logging look? Are there any changes that I should make to make this more consistent with Compass behavior?
Merge branch 'main' into extend-user-data
Merge branch 'extend-user-data' of https://github.com/mongodb-js/compass into extend-user-data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gribnoysup @Anemy
Do you guys have any clue where these may have come from? I haven't personally modified any of them, so I wonder whether it's due to command line commands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, those are not really related. Probably accidentally run a compile command across all packages? We haven't updated this in a while, so I guess there might be some slight formatting variations or something, probably better to just revert those from your patch
@@ -6,9 +6,8 @@ export class HistoryStorage { | |||
userData; | |||
|
|||
constructor(basePath?: string) { | |||
this.userData = new FileUserData(z.string().array(), { | |||
this.userData = new FileUserData(z.string().array(), getAppName() ?? '', { | |||
// Todo: https://jira.mongodb.org/browse/COMPASS-7080 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this todo should probably still point at the getAppName()
part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just referring to the TODO comment?
a7d1e33
to
f6efcd6
Compare
Remove unwanted bson-transpilers changes that were accidentally included in the rebase. This resets the bson-transpilers package back to its master state while preserving the user-data related changes that are needed for this PR.
Rebuild the generated symbol table files to ensure they are properly formatted and up-to-date with the current source templates.
Trying to figure out if there are more outstanding comments here that I'll need to resolve as I'm finishing up this PR for Moses! This PR can be merged without fear (I believe) since the actual storage providers don't get added until the next PR. Let me know if that understanding is incorrect! |
Description
Extend the IUserData abstract class to create an AtlasUserData class that uses the endpoints created in CCS. Contains unit testing.
Checklist
Motivation and Context
Open Questions
See comments
Dependents
Types of changes